Thank you for this PR @CelsoDeCarvalho ! It generally looks good. Could you change it to use path.parse(process.cwd()).root on all operating systems instead of only on Windows? The same should work on *nix machines as well.

It is unnecessary to use path.parse(process.cwd()).root for non windows OS, because it is known that the root will be "/" I think. It will help us to avoid invoke parse and cwd function unnecessarly.

Do you think that I needa change it?

Thanks Celso - thinking about this a little more, I think using process.cwd() actually might not always be correct. If a directory with another root (like D://) is passed as the base directory it will never be found. Also the way this change is currently implemented the root is computed recursively but will never change, so it would be a bit more efficient to only do this calculation once.

I would recommend instead changing this line to findYarnLock(base, path.parse(base).root) and then updating the findYarnLock function to accept both base and root as parameters, and passing both when calling it recursively.

Let me know if this makes sense to you, thanks again for helping to fix this!

Hi @CelsoDeCarvalho thanks again for your contribution here! Do you think you will have time to apply the changes requested? I'd like to get this fixed, so if you don't have time I can also do it myself and merge a separate PR. Let me know!

Thanks @amcgee for text me. Oh! I had forgotten about this point. I'll make changes today. Thanks

It will be a pleasure to contribute to this repo.

Thanks Celso - thinking about this a little more, I think using process.cwd() actually might not always be correct. If a directory with another root (like D://) is passed as the base directory it will never be found. Also the way this change is currently implemented the root is computed recursively but will never change, so it would be a bit more efficient to only do this calculation once.

I would recommend instead changing this line to findYarnLock(base, path.parse(base).root) and then updating the findYarnLock function to accept both base and root as parameters, and passing both when calling it recursively.

Let me know if this makes sense to you, thanks again for helping to fix this!

@amcgee This make sense. The variable declaration inside a recursive function can decrease the perfomance. I have made the changes. But for the first point, can you please explain it?
Thanks.

@CelsoDeCarvalho thanks and certainly!

Basically, the variable cwd here will default to process.cwd() (the current working directory from which this command is run) but it can be overridden with a command line argument. So it would be possible to do this:

> cd D:\test\folder
> d2 app scripts init --cwd C:\development\apps mynewapp

In this case, process.cwd() would be on the D:\ drive but the cwd argument (which indicates where the new app should be created) would be on the C:\ drive. Since we're starting from the cwd directory and traversing up the file system heirarchy to find the root directory, we'd eventually get to C:\. If we used path.parse(process.cwd()).root instead of path.parse(base).root it would try to match D:\ as the eventual base condition to terminate the recursion, but that would never happen. So we need to make sure that we're finding the root directory of the directory we're working on which might not be the current working directory in bash. Does that make sense?

This change looks good to me now! I made one very minor suggestion to update the naming of the root variable. Thanks again for your contribution!

Don't worry about the commit message lint issue - it's caused by the fact that several commits here aren't exactly following conventional commit standards - 6b61c07 is missing a : and 201b22a is missing a (space) after the :.

I will squash-merge this branch when I merge it, so no need to fix those on your branch :-)

@CelsoDeCarvalho thanks and certainly!

Basically, the variable cwd here will default to process.cwd() (the current working directory from which this command is run) but it can be overridden with a command line argument. So it would be possible to do this:

> cd D:\test\folder
> d2 app scripts init --cwd C:\development\apps mynewapp

In this case, process.cwd() would be on the D:\ drive but the cwd argument (which indicates where the new app should be created) would be on the C:\ drive. Since we're starting from the cwd directory and traversing up the file system heirarchy to find the root directory, we'd eventually get to C:\. If we used path.parse(process.cwd()).root instead of path.parse(base).root it would try to match D:\ as the eventual base condition to terminate the recursion, but that would never happen. So we need to make sure that we're finding the root directory of the directory we're working on which might not be the current working directory in bash. Does that make sense?

This change looks good to me now! I made one very minor suggestion to update the naming of the root variable. Thanks again for your contribution!

Hi @amcgee

So Lemme see if I get it. Basically you saying that there will be possible to create app in a directory passed through the command line in --cwd flag and some times if the directory am passing be diferent of my current working directory, the recursion stop condition will not be verified, rite? And we have done that point 'cause we're using cwd variable that I think this is the one that will be used as the command line argument. Please let me know if am right. Thanks

I have made the changes.

@CelsoDeCarvalho thanks and certainly!
Basically, the variable cwd here will default to process.cwd() (the current working directory from which this command is run) but it can be overridden with a command line argument. So it would be possible to do this:

> cd D:\test\folder
> d2 app scripts init --cwd C:\development\apps mynewapp

In this case, process.cwd() would be on the D:\ drive but the cwd argument (which indicates where the new app should be created) would be on the C:\ drive. Since we're starting from the cwd directory and traversing up the file system heirarchy to find the root directory, we'd eventually get to C:\. If we used path.parse(process.cwd()).root instead of path.parse(base).root it would try to match D:\ as the eventual base condition to terminate the recursion, but that would never happen. So we need to make sure that we're finding the root directory of the directory we're working on which might not be the current working directory in bash. Does that make sense?
This change looks good to me now! I made one very minor suggestion to update the naming of the root variable. Thanks again for your contribution!

Hi @amcgee

So Lemme see if I get it. Basically you saying that there will be possible to create app in a directory passed through the command line in --cwd flag and some times if the directory am passing be diferent of my current working directory, the recursion stop condition will not be verified, rite? And we have done that point 'cause we're using cwd variable that I think this is the one that will be used as the command line argument. Please let me know if am right. Thanks

I have made the changes.

That's correct, thank you @CelsoDeCarvalho !

I manually built, tested, and linted this PR before merging. Thanks again @CelsoDeCarvalho for the contribution!!

🎉 This PR is included in version 10.3.10 🎉

The release is available on:

• npm package (@latest dist-tag)
• npm package (@latest dist-tag)
• npm package (@latest dist-tag)
• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

🎉 This PR is included in version 10.4.0-alpha.4 🎉

The release is available on:

• npm package (@alpha dist-tag)
• npm package (@alpha dist-tag)
• npm package (@alpha dist-tag)
• npm package (@alpha dist-tag)
• GitHub release

Your semantic-release bot 📦🚀